Skip to content

fix: reconcile pydantic v2 config names and drop unused fields - #195

Merged
dimoschi merged 6 commits into
mainfrom
fix/config-env-consistency
Oct 6, 2026
Merged

dimoschi merged 6 commits into
mainfrom
fix/config-env-consistency

Conversation

@ToxicBiohazard

Copy link
Copy Markdown
Member

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply.

  • Bugfix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality not to work as expected).
  • Documentation Update (if none of the other choices applies).

Proposed changes

Reconcile the config leftovers from the Pydantic v2 migration.

Fixes #189
Fixes #190

  • Remove settings that nothing reads: APRIL_ROLE_1, APRIL_ROLE_2, RANK_FIVE, RANK_TWENTY_FIVE, RANK_FIFTY, RANK_HUNDRED, ACADEMY_CBBH, APRIL_FLAG_1, APRIL_FLAG_2, SLACK_WEBHOOK, and JIRA_SPOILER_WEBHOOK. extra="forbid" will reject them if they are still set.
  • Seed dynamic roles from ROLE__{suffix} instead of the pre-migration ROLE_ prefix.
  • Choose the settings file with APP_ENV_FILE. BOT_ENVIRONMENT still works, so current Vault configs keep booting. Either flag loads ENV_PATH when it is set.
  • Build the database URL in one place. ASYNC selects asyncmy or pymysql, and Alembic calls that builder with the sync driver instead of reading MYSQL_*.

Checklist

Put an x in the boxes that apply.

  • I have read and followed the CONTRIBUTING.md doc.
  • Lint and unit tests pass locally with my changes.
  • I have added the necessary documentation (if appropriate).

Additional context

startup.sh still uses MYSQL_* for the readiness ping. .docker.env already defines both MYSQL_* and DATABASE__*.

Before deploy, Vault still needs:

  • SEASON_ID set to the real season. .test.env already has it. A local .env may still only have CURRENT_SEASON_ID.
  • The removed keys deleted, or startup fails. That includes JIRA_SPOILER_WEBHOOK if it is still present.

Made with Cursor

ToxicBiohazard and others added 4 commits September 29, 2026 07:22
These fields are never read. Leaving them in place hides stale Vault keys that extra=forbid should reject.

Co-authored-by: Cursor <[email protected]>
The seed script still looked up the pre-migration ROLE_ prefix, so it skipped every role in the current env files.

Co-authored-by: Cursor <[email protected]>
BOT_ENVIRONMENT is one character away from BOT__ENVIRONMENT, and they mean different things. APP_ENV_FILE now selects the deployed env file, while the old name still works.

Co-authored-by: Cursor <[email protected]>
Alembic still read the pre-migration MYSQL_* variables and ignored ASYNC. Migrations now use the same URL builder as the app, with the sync driver forced on.

Co-authored-by: Cursor <[email protected]>
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.57%. Comparing base (76b2f25) to head (15f9f0a).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
scripts/seed_dynamic_roles.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #195      +/-   ##
==========================================
- Coverage   66.60%   65.57%   -1.03%     
==========================================
  Files          54       77      +23     
  Lines        3177     3788     +611     
==========================================
+ Hits         2116     2484     +368     
- Misses       1061     1304     +243     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The settings read grouped values as BOT__*, CHANNEL__* and ROLE__* since
the Pydantic v2 migration, but the README still listed BOT_TOKEN and
CHANNEL_DEVLOG.

@dimoschi dimoschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup, and it merges cleanly on current main (all tests pass, alembic still builds its URL fine).

One blocker: prod and dev still have some of the removed ROLE__* keys, so with extra="forbid" the bot would not start after this deploys. Details inline. The Vault keys need to go anyway, but the app should not crash on them.

Also, dev has CURRENT_SEASON_ID but no SEASON_ID, so it runs with season 0. Worth adding it there too.

Comment thread src/core/config.py
Comment thread src/core/config.py Outdated
Comment thread src/core/config.py Outdated
Removed ROLE__ keys are still in Vault, and extra=forbid would stop the bot from starting. The app always uses the async driver and Alembic always uses the sync one. APP_DEPLOYED is the deploy flag, and CURRENT_SEASON_ID still fills SEASON_ID.

Co-authored-by: Cursor <[email protected]>

@dimoschi dimoschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all good now. I checked that it boots with the old keys we still have in prod, and that CURRENT_SEASON_ID fills SEASON_ID when it's the only one set. Tests pass on top of current main.

@dimoschi
dimoschi merged commit 2a7c550 into main Oct 6, 2026
4 checks passed
@dimoschi
dimoschi deleted the fix/config-env-consistency branch October 6, 2026 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: reconcile config naming inconsistencies from Pydantic v2 migration chore: remove unused config fields from v1.13.0 pydantic migration

2 participants